Implement Chart Style Options Menu#378
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the card component architecture by shifting the responsibility of rendering actions, content, and footers from the shape utility down to the specific card variants (Card.Text and Card.Chart). It also introduces a new MenuChartOptions component to support switching chart styles. The review feedback highlights a few issues: a missing ref attachment on the dialog container which breaks the focus trap, a missing auto-close behavior when confirming a chart style, and a regression where an undefined card variant no longer falls back to rendering text.
| const [selectedStyle, setSelectedStyle] = | ||
| useState<ChartStyle>('bar-vertical'); | ||
| const [isStyleMenuOpen, setIsStyleMenuOpen] = useState(false); | ||
|
|
There was a problem hiding this comment.
If you open up the chart menu and click within the chart, it dismisses it (catching the click within the card but outside the menu).
However, if you click anywhere outside the card, the card becomes deselected but the menu remains.
Noticing the deselection happens, we could fix this in a quick way by adding an effect that catches the change of selection:
useEffect(() => {
if (selection === 'none') {
setIsStyleMenuOpen(false);
}
}, [selection]);
It does create a bespoke "chain reaction" to solve the issue.
A more generic clean option could be to use the useClickOutside hook that we have in the use_click_outside (and we cwuldn't then need the backdrop, although it could still serve a visual purpose over the card.
2 participants
Overview
Adds the chart-style options menu to chart cards, reachable from the card's action bar.
What's new
Refactor
Screenshot